Skip to content

gfx: scene-concept rework (the coupled core of #2109) - #2120

Open
jcelerier wants to merge 16 commits into
planc/gfx-infrafrom
pland/scene
Open

gfx: scene-concept rework (the coupled core of #2109)#2120
jcelerier wants to merge 16 commits into
planc/gfx-infrafrom
pland/scene

Conversation

@jcelerier

Copy link
Copy Markdown
Member

The scene-concept rework — the coupled core of #2109. This is the part that couldn't be decomposed further: a scene-aware render pipeline plus the GPU-resident scene state, node-lifecycle, and per-backend plumbing that all change together. Everything separable was already pulled into #2117/#2118/#2119 to make this PR as small as it can be.

Rebased onto current master (auto-merges cleanly with master's newer nodal-selection work). Stacked on #2119; retargets to master as the stack merges.

Commits

  • gfx: add the OffsetAllocator submodule (arena backing for GPU-resident scene data).
  • gfx: rework the render pipeline to be scene-aware and incrementally rebuilt — SceneGPUState, the GpuResourceRegistry arena, incremental RenderList rebuild, and the QRhi pipeline cache.
  • gfx: add the scene preprocessor and scene filter nodes.
  • avnd: make the CPU and GPU node lifecycles scene-aware.
  • js: rework the GPU node lifecycle with deterministic teardown.
  • threedim: adapt ModelDisplay to the new pipeline API.

This wires up the caching/offscreen infrastructure landed in #2119.

Validation

  • Builds cleanly against current master (all plugins).
  • ctest: 17/17.
  • Process + device factory sweeps under ASAN + UBSan (the sweeps construct and round-trip every reworked gfx node model): 0 memory errors, 0 undefined-behavior in score's own code.

🤖 Generated with Claude Code

@jcelerier
jcelerier force-pushed the pland/scene branch 2 times, most recently from 6135c95 to 726e579 Compare July 17, 2026 11:58
@jcelerier
jcelerier force-pushed the planc/gfx-infra branch 2 times, most recently from 1b6651e to 08516ad Compare July 17, 2026 14:10
jcelerier added a commit that referenced this pull request Jul 17, 2026
FIX-ISF (libisf/src/isf.cpp — applies on top of this branch's ISF/CSF
feature rework, hence #2120 not #2119):
 - write_isf: seekp(0,end)-before-tellp corrupted the emitted descriptor
 - use-after-move on m_sourceVertex -> guard with .empty()
 - parse_shadertoy_json: GLSL45 preludes + iMouse vec4(0,0,0,0)
 - glsl_sandbox: duplicate-TIME uniform guard
 - replace_identifier: word-boundary match

Tests: test_unit_isf_importers (all 5 importer branches); golden-image
render regression (16 pinned JS-corpus cases, llvmpipe, SSIM/PSNR);
timeline-driven scenario ramp; gfx resource/leak soak (DISABLED — guards
a still-unfixed render-clock teardown UAF); live-edit churn corpus.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
jcelerier added a commit that referenced this pull request Jul 17, 2026
FIX-ISF (libisf/src/isf.cpp — applies on top of this branch's ISF/CSF
feature rework, hence #2120 not #2119):
 - write_isf: seekp(0,end)-before-tellp corrupted the emitted descriptor
 - use-after-move on m_sourceVertex -> guard with .empty()
 - parse_shadertoy_json: GLSL45 preludes + iMouse vec4(0,0,0,0)
 - glsl_sandbox: duplicate-TIME uniform guard
 - replace_identifier: word-boundary match

Tests: test_unit_isf_importers (all 5 importer branches); golden-image
render regression (16 pinned JS-corpus cases, llvmpipe, SSIM/PSNR);
timeline-driven scenario ramp; gfx resource/leak soak (DISABLED — guards
a still-unfixed render-clock teardown UAF); live-edit churn corpus.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
jcelerier added a commit that referenced this pull request Jul 17, 2026
Headless llvmpipe golden+value harness for the Gfx::Text process: grabs the
untouched-defaults frame (hard off-screen-default visibility assertion — guards
the #2119 default-position fix), then live-edits text/font/size/position/scale/
color over OSC /script and asserts pixel VALUES per case (coverage, bbox
ordering across sizes, centroid movement, channel dominance, blank empty
string, unicode/CJK/tofu/2000-char) + golden refs (compare.py strict).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
@jcelerier
jcelerier force-pushed the pland/scene branch 2 times, most recently from 019649f to cb34e12 Compare July 18, 2026 13:37
@jcelerier
jcelerier force-pushed the planc/gfx-infra branch 2 times, most recently from 2676d67 to 387b649 Compare July 18, 2026 14:17
@jcelerier
jcelerier force-pushed the pland/scene branch 2 times, most recently from bd66e34 to 0da070a Compare July 19, 2026 21:59
@jcelerier
jcelerier force-pushed the pland/scene branch 4 times, most recently from 9e6753d to f4015d0 Compare July 22, 2026 06:22
jcelerier added a commit that referenced this pull request Aug 16, 2026
One process per case, so a case that crashes or hangs takes only itself down,
and each tester builds its document from a clean application either way.

Verdicts are separated by what actually happened rather than lumped into
pass/fail: RENDER, BLANK, NORENDER, SCREEN, FAIL. SCREEN exists because a grab
that falls back to capturing the desktop is not a weaker result, it is a
meaningless one, and the sweep exits nonzero if any case produced one.

First run, 130 cases: 81 render on the stack against 60 on master, the
difference being shaders whose ISF features only exist above #2120 -- the
measured form of the claim that no pixel test can run below it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
jcelerier added a commit that referenced this pull request Aug 16, 2026
One process per case, so a case that crashes or hangs takes only itself down,
and each tester builds its document from a clean application either way.

Verdicts are separated by what actually happened rather than lumped into
pass/fail: RENDER, BLANK, NORENDER, SCREEN, FAIL. SCREEN exists because a grab
that falls back to capturing the desktop is not a weaker result, it is a
meaningless one, and the sweep exits nonzero if any case produced one.

First run, 130 cases: 81 render on the stack against 60 on master, the
difference being shaders whose ISF features only exist above #2120 -- the
measured form of the claim that no pixel test can run below it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
@jcelerier
jcelerier force-pushed the pland/scene branch 2 times, most recently from 671136f to 119e5ef Compare August 16, 2026 19:57
@jcelerier
jcelerier force-pushed the pland/scene branch 2 times, most recently from 4e0f9ff to 29bed8a Compare August 17, 2026 13:30
jcelerier and others added 16 commits August 19, 2026 16:20
Bring in OffsetAllocator to back the slab-allocated GPU arenas.

(cherry picked from commit 5b7c7fb)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
…ebuilt

The core scene-GPU rework. Adds the supporting infrastructure — OffsetAllocator-backed
GPU arenas (GpuResourceRegistry), a shared decoded-asset cache (AssetTable +
TextureLoader), scene math and pipeline-state helpers, the flat scene state and packer
(SceneGPUState), an offscreen RHI device and preview widget — then reworks the pipeline
on top of it: the ISF parser and nodes gain 3D samplers, uniform inputs and
geometry/vertex stages; the compute and raster pipelines become scene-aware with multiple
render targets and auxiliary outputs; the render graph updates edges, render targets and
passes in place instead of rebuilding wholesale; and the filter, texture-port, window,
capture and video-output paths follow the new node and renderer interfaces.

(cherry picked from commit b824245)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
ScenePreprocessorNode turns a scene spec into flat draw commands and arena uploads;
SceneFilterNode, FlattenedSceneFilterNode and MergeGeometriesNode operate on flattened
scenes.

(cherry picked from commit f7bdb1d)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
(cherry picked from commit 8b2c463)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
buildPipeline returns a null pipeline when QRhiGraphicsPipeline::create()
fails (transient during graph rebuild). InvertYRenderer::finishFrame
dereferenced it via setGraphicsPipeline (Q_ASSERT/null-deref), and
quadRenderPass asserted on the then-missing pass. Both now skip the draw,
matching defaultRenderPass / the if(pip.pipeline) guard at pass creation.

(cherry picked from commit 1290424)
libisf/src/isf.cpp:
 - write_isf: seekp(0,end)-before-tellp corrupted the emitted descriptor
 - use-after-move on m_sourceVertex -> guard with .empty()
 - parse_shadertoy_json: GLSL45 preludes + iMouse vec4(0,0,0,0)
 - glsl_sandbox: duplicate-TIME uniform guard
 - replace_identifier: word-boundary match

Tests: test_unit_isf_importers (all 5 importer branches); golden-image
render regression (16 pinned JS-corpus cases, llvmpipe, SSIM/PSNR);
timeline-driven scenario ramp; gfx resource/leak soak (DISABLED — guards
a still-unfixed render-clock teardown UAF); live-edit churn corpus.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
Headless llvmpipe golden+value harness for the Gfx::Text process: grabs the
untouched-defaults frame (hard off-screen-default visibility assertion — guards
the default-position fix), then live-edits text/font/size/position/scale/
color over OSC /script and asserts pixel VALUES per case (coverage, bbox
ordering across sizes, centroid movement, channel dominance, blank empty
string, unicode/CJK/tofu/2000-char) + golden refs (compare.py strict).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
destroyOutput() is the synchronous counterpart of the async REMOVE_NODE path,
which already calls removeNodeAndEdges. Without it, Graph::removeNode leaves
m_edges holding Edges that point at the freed output's Ports, so ~Graph ->
clearEdges() unlinks them from freed memory.

Also finish the render-clock conversion in the destructor: the clocks must be
released before the timer pool they borrow from.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
gl_ClipDistance and gl_CullDistance are built-ins already declared in the
gl_PerVertex block, so emitting `out float gl_ClipDistance[N];` as a bare global
is a redeclaration that changes their qualification. The compiler says exactly
that — "cannot change qualification of gl_ClipDistance" — and every shader using
the feature failed to build. Redeclare the block instead, which is the form the
spec provides for sizing them; it has to carry gl_Position too, because
redeclaring a built-in block replaces it.

Also stop handing Qt a null render target: runInitialPasses picks rtForPass from
a chain of per-mip / per-face / per-layer branches, each guarded on the target
existing, so a pass that matches none of them reached beginPass with nullptr and
segfaulted inside QRhi. Skip the pass and say so, the same way createRenderTarget
degrades rather than aborting.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An ISF uniform_input is a VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER, so the buffer
behind it must carry UniformBuffer usage. Cabling a producer that publishes a
storage buffer -- any `storage` RESOURCE exposes a Types::Buffer output -- is a
graph a user can build, and we were binding it: vkUpdateDescriptorSets then
rejects the write with VUID-VkWriteDescriptorSet-descriptorType-00330 and the
next setShaderResources segfaults. OpenGL has no descriptor sets, so nothing
caught the mismatch there and the shader read whatever that binding exposed.

Check the usage and keep the zero-filled placeholder instead, with a warning
naming the input. The invalid graph no longer renders correctly -- a storage
buffer is not a UBO -- but it is now defined and diagnosable on every backend
rather than a crash on one and silent garbage on the other.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Covers 5ed56c9 (pland/scene). flattenScene / packMaterial /
primitiveToGeometry / packCameraUBO / setReverseZPerspective are pure
functions over plain ossia::scene_spec data and had no test anywhere in the
tree, while a sign or row/column slip in the reverse-Z projection silently
inverts depth across the whole renderer — geometry still draws, it just
occludes backwards, which is exactly what a "did anything render" sweep
cannot see.

SceneGPUState.cpp and CameraMath.cpp are compiled straight into the test
targets: score_plugin_gfx is built with hidden visibility and exports
neither. This is the workaround tests/gfx/CMakeLists.txt already applies to
ISFNode.cpp et al; src/ is not modified.

CameraUBOData's field offsets are pinned, not only its size: two out-of-tree
tester shaders encode a 208-byte camera entry against this 240-byte one, so
the ABI drift is live rather than hypothetical.

Two findings recorded as assertions rather than fixed:

  - FlatScene's doc-comment says the no-camera fallback eye is (0,1,3);
    SceneGPUState.cpp writes (0,0,3). The test pins the code.
  - FlatScene::clear() resets the containers and the two flags but not
    viewMatrix / projectionMatrix / cameraPosition / cameraFov / cameraNear /
    cameraFar, and the empty-scene path returns before the fallback block
    would rewrite them, so a reused FlatScene keeps the previous scene's
    camera on it. hasCamera is the only reliable signal, and the test says so.

Negative controls, each seen red before being trusted:
  - parentWorld * xform -> xform * parentWorld makes the parent-on-the-left
    case red (1.0 instead of 2.0). A translation-only hierarchy cannot catch
    this — translations commute — so that case uses a scale.
  - dropping the raw_slot.size guard on the light arena slot makes the
    producer-less sentinel case red (0 instead of 0xFFFFFFFF).
  - negating out(2,2)/out(2,3) in setReverseZPerspective turns 203 of the
    286 camera assertions red.

(cherry picked from commit 9d782ea920cbd63a3140b0f55be54adb12db30aa)
Covers 45d1202 (pland/scene). ScenePreprocessor's packAndUploadCameras
packs one CameraUBOData per camera the flattener collected into
m_camerasBuffer — capacity at least 16 of them — and puts the active camera
at slot 0. Both publication sites then advertised the `camera` auxiliary's
byte_size as a bare sizeof(CameraUBOData): 240 bytes, exactly one entry,
however many were packed. A MULTIVIEW shader indexing camera[gl_ViewIndex]
over six cubemap faces reads outside the range its binding declares —
undefined, and on Vulkan a validation error rather than the intended faces.

The engine is NOT fixed here. What the two literals become is one named
inline function, cameraAuxByteSize(cameraCount), called from both sites with
the count that was actually packed; today it ignores its argument and returns
sizeof(CameraUBOData), so the generated code is unchanged. That is the whole
production diff, and it exists so the contract has somewhere to be asserted
and, later, one place to be fixed.

test_unit_scene_camera_aux is its own target, in the style of
test_gfx_isf_findings, so an attributable RED cannot take down the flattener
suite next door.

  the camera auxiliary covers every camera the flattener packed
    CHECK( cameraAuxByteSize(out.cameras.size())
           == (int64_t)(out.cameras.size() * sizeof(CameraUBOData)) )
    with expansion: 240 == 1440 (0x5a0)

The single-camera case passes, which is why nothing has noticed. Making
cameraAuxByteSize return max(1, count) * sizeof(CameraUBOData) turns the whole
target green, so this is a gate on the defect and not merely a broken
assertion — the mesh path's wrapGpu(m_camerasBuffer, sizeof(CameraUBOData))
slice has to grow with it.

(cherry picked from commit 377dbc1ee8b0c71ec2319112ac8f822b46d309a8)
Covers 5ed56c9 (pland/scene), which introduced PIPELINE_STATE and with it
a DEPTH_COMPARE clause accepting both compare directions, while the draw
passes clear depth to a hardcoded 0.0 —
RenderedRawRasterPipelineNode.cpp's beginPass and RenderList.cpp's node-pass
beginPass. 0.0 is what the project-wide reverse-Z convention wants
(CameraMath.hpp: D32F + GREATER + clear 0.0). Depth after the viewport
transform is in [0, 1], so under `less` that clear admits no fragment at all
and under `less_equal` only the single plane at exactly 0.0: the first draw
into the target is entirely rejected and the frame stays at the clear colour,
with no diagnostic. tests-scene's ps-depth-test.fs declares exactly that, and
ShaderSweepScene records "nothing drawn" as an unasserted observation in a
file that SKIPs in every clean checkout.

The engine is NOT fixed here. The two literals become one exported function,
depthClearForCompare(compare), called from both sites with the convention's
own Greater; today it ignores its argument and returns 0.0f, so the generated
code is unchanged. That is the whole production diff, and it is the seam the
fix goes through.

The test states the property rather than the value: a (clear, compare)
pairing is usable when every depth sampled over [0, 1] except at most the
clear plane itself draws into a freshly cleared target. Under that predicate
`greater`, `greater_equal` and `always` pass, `never` correctly does not, and:

  a shader declaring a less compare gets a clear it can pass
    CHECK( clearIsUsableBy(toCompareOp(s)) )  ->  false   [DEPTH_COMPARE: less]
    CHECK( clearIsUsableBy(toCompareOp(s)) )  ->  false   [less_equal]
    CHECK( clearIsUsableBy(toCompareOp(s)) )  ->  false   [lequal]

  the depth clear differs between the two compare directions
    CHECK( depthClearForCompare(Less) != depthClearForCompare(Greater) )
    with expansion: 0.0f != 0.0f

Returning 1.0f for Less/LessOrEqual turns the target green, so this gates the
defect rather than merely asserting something false.

(cherry picked from commit de28d4db9ebea2bf6d23ae23b6af98f4038b59f0)
Covers e6bf5d7 (pland/scene). CLIP_DISTANCES / CULL_DISTANCES were emitted
as bare globals — `out float gl_ClipDistance[N];` — but both built-ins are
already declared inside gl_PerVertex, so that is a redeclaration changing
their qualification and glslang rejects it: "cannot change qualification of
gl_ClipDistance". Every shader using the feature failed to build, and nobody
noticed, because the strings CLIP_DISTANCES / CULL_DISTANCES / gl_PerVertex
appear nowhere under tests/ and no corpus shader declares either.

Four cases, all parser-level, no GPU and no app, in the file that already
owns the libisf parser surface. The load-bearing one is negative:
CHECK(!contains(vert, "out float gl_ClipDistance")) — that is what the
pre-fix emitter produces.

The clamp behaviour is asserted as the code has it, not as the commit
message describes it: isf.cpp accepts 1..8 and otherwise leaves the count at
zero, so an out-of-range count emits no block at all rather than a clamped
one.

Negative control: restoring the bare-global emission in isf.cpp turns 8
assertions across 2 of the 4 cases red, including both `!contains` guards.

(cherry picked from commit 1671fcba51d47e6b638a1196ad343947e641ada9)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants